Skip to content

Updates to build on Windows#191

Merged
dmcgowan merged 6 commits into
containerd:mainfrom
dmcgowan:windows-build-changes
May 15, 2026
Merged

Updates to build on Windows#191
dmcgowan merged 6 commits into
containerd:mainfrom
dmcgowan:windows-build-changes

Conversation

@dmcgowan
Copy link
Copy Markdown
Member

Multiple changes to get nerdbox building and running on Windows

djs55 and others added 5 commits May 14, 2026 23:36
Call krun_free_ctx (via vmc.Shutdown) before unloading the library with
dlClose/FreeLibrary. krun_free_ctx is synchronous: it joins all vCPU and
virtio worker threads and closes all file handles held by the VM.

Previously, dlClose was called without stopping the VM first. On Unix
this was mostly harmless (the process was exiting anyway), but on Windows
FreeLibrary immediately unloads the DLL while threads are still running
against it, and leaves file handles open. The open handles prevent
containerd from renaming/deleting the bundle directory, which blocks
container restart.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Derek McGowan <derek@mcg.dev>
Remove the bundle's rootfs directory during graceful shim shutdown,
before containerd's bundle cleanup runs.

On Windows, containerd's UnmountAll calls bindfilter.RemoveFileBinding
on the rootfs directory. This fails with ERROR_ACCESS_DENIED because
nerdbox never sets up a bind filter mount — it passes the rootfs into
the VM via virtio block devices. The rootfs directory in the bundle is
just an empty mount point that containerd created.

Removing it before cleanup makes UnmountAll a no-op, allowing the
bundle to be deleted and the container to restart.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Derek McGowan <derek@mcg.dev>
Fix three issues in the Windows manager Stop() method, which runs in
the delete subcommand after a shim disconnects:

1. Use absolute bundle path for file operations. On Windows, containerd
   deliberately does not set the delete subcommand's CWD to the bundle
   directory (to avoid holding an implicit directory handle that would
   block cleanup). The previous code used relative paths like
   "shim.pid", which resolved against the wrong directory. Extract the
   bundle path from the context (-bundle flag) and use it explicitly.

2. Wait for the shim process to exit before returning. After reading
   the shim PID, wait for the process to fully terminate (up to 10s)
   so that all file handles — from the VM, krun DLL, virtio devices,
   and the shim's own CWD — are released. Without this, containerd's
   bundle rename races with handle cleanup and fails with
   ERROR_SHARING_VIOLATION.

3. Remove the rootfs directory before returning, same as commit
   "shim/task: remove rootfs directory during shutdown" does for the
   graceful path. This handles the case where the shim was killed
   before its shutdown handler ran (e.g. SIGKILL after stop timeout),
   or where the shim's cleanup races with containerd's under load.

Also handle the case where shim.pid does not exist: the shim already
exited and cleaned up. Return a synthetic exit status instead of
failing, and still remove the rootfs directory.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Derek McGowan <derek@mcg.dev>
Prevents CRLF issues with heredoc shell scripts on Windows.

Signed-off-by: Derek McGowan <derek@mcg.dev>
Falls back to non-suffixed libkrun names for system-installed libraries.

Signed-off-by: Derek McGowan <derek@mcg.dev>
Copilot AI review requested due to automatic review settings May 15, 2026 06:45
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Multiple changes to support building and running nerdbox on Windows: arch-suffixed libkrun/initrd asset discovery, synchronous libkrun shutdown so DLL unload doesn't strand threads/handles, and Windows-friendly bundle cleanup that avoids containerd's bind-filter unmount path.

Changes:

  • Add architecture-suffixed candidate names for libkrun shared objects and the initrd; update the "not found" error accordingly.
  • Call vmc.Shutdown() (krun_free_ctx) before dlClose so file handles/threads are released.
  • Remove the bundle's rootfs dir to make containerd's UnmountAll a no-op on Windows; resolve shim.pid via the bundle path passed in the context; add a .gitattributes rule forcing LF for Dockerfile.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
internal/vm/libkrun/instance.go Adds arch-suffixed library/initrd lookups and a synchronous VM shutdown before unloading libkrun.
pkg/shim/manager/manager_windows.go Reads shim.pid from the bundle path and removes rootfs to avoid Windows bind-filter unmount errors.
internal/shim/task/service.go Removes rootfs during shim shutdown to bypass containerd's UnmountAll on the bundle.
.gitattributes Forces LF line endings for Dockerfile.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread internal/shim/task/service.go Outdated
Comment thread pkg/shim/manager/manager_windows.go
- vm/libkrun: fall back to 'nerdbox-initrd' when arch-suffixed file not
  found, fixing integration tests on CIs that still ship the old name;
  update the not-found error to mention both candidates
- shim/task: move rootfs removal into platform-specific files so it only
  runs on Windows (bind-filter workaround), not on Linux/macOS where
  removing through a live mountpoint can corrupt data
- shim/manager: write shim.pid via bundle path in Start() so it is
  consistent with Stop() which already reads it from the bundle path

Signed-off-by: Derek McGowan <derek@mcg.dev>
@dmcgowan dmcgowan merged commit 67ac87e into containerd:main May 15, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants